Skip to content

OPDATA-7692 filter endpoints by env var initial commit#804

Open
mmcallister-cll wants to merge 5 commits into
mainfrom
OPDATA-7692-filter-endpoints
Open

OPDATA-7692 filter endpoints by env var initial commit#804
mmcallister-cll wants to merge 5 commits into
mainfrom
OPDATA-7692-filter-endpoints

Conversation

@mmcallister-cll
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

NPM Publishing labels 🏷️

🟢 This PR has valid version labels and will cause a patch bump.

@mmcallister-cll mmcallister-cll marked this pull request as ready for review June 4, 2026 19:39
@mmcallister-cll mmcallister-cll requested a review from a team as a code owner June 4, 2026 19:39
const makeEndpoint = (name: string) => new AdapterEndpoint({ name, transport: new NopTransport() })

test.afterEach(() => {
delete process.env['ENABLED_ENDPOINTS']
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup should go in beforeEach. If a test fails to clean up, you want that to fail, not some random next test.

This is more of an issue with jest where you can have before/afterEach apply to subsets of tests, but still a good rule of thumb to follow.

afterEach is only for assertions like "no errors happened during the test", or for cleanups that are not allowed to run initially.

Comment thread src/adapter/basic.ts
* Filters the adapter's endpoints based on the ENABLED_ENDPOINTS setting.
* If ENABLED_ENDPOINTS is not set, all endpoints are loaded.
* If ENABLED_ENDPOINTS is set, only the specified endpoints are loaded.
* Throws an error if none of the specified endpoints match.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should throw if any of the listed endpoints don't match. That would be a mistake that you want to find out sooner rather than later.

endpoints: [makeEndpoint('price'), makeEndpoint('volume')],
})
t.is(adapter.endpoints.length, 1)
t.is(adapter.endpoints[0].name, 'price')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combine these 2 lines into

t.deepEqual(adapter.endpoints.map(e => e.name), ['price'])

The error will be more useful than just the length not matching.

name: 'TEST',
endpoints: [makeEndpoint('price'), makeEndpoint('volume')],
})
t.is(adapter.endpoints.length, 2)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants